[Flight] model halted references explicitly #30731
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
React Core Team
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
00f030d
to
80aaeae
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
80aaeae
to
eda582e
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
eda582e
to
78a72fc
Compare
gnoff
commented
Aug 17, 2024
unstubbable
reviewed
Aug 17, 2024
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
78a72fc
to
2d91d08
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
2d91d08
to
3ae48a8
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
3ae48a8
to
ea1bd79
Compare
sebmarkbage
reviewed
Aug 18, 2024
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
ea1bd79
to
925fba5
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
925fba5
to
5b582d3
Compare
sebmarkbage
reviewed
Aug 18, 2024
sebmarkbage
reviewed
Aug 18, 2024
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
5b582d3
to
633305c
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
633305c
to
2ddbeca
Compare
sebmarkbage
reviewed
Aug 19, 2024
| function allReady(request: Request) { | ||
| const onAllReady = request.onAllReady; | ||
| onAllReady(); | ||
| } |
There was a problem hiding this comment.
If we unify the rest of halt/abort, then this helper doesn't become necessary.
There was a problem hiding this comment.
I'll do in the next where we re-add the error and postpone logging
sebmarkbage
approved these changes
Aug 19, 2024
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
2ddbeca
to
dec9897
Compare
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
dec9897
to
35b6d73
Compare
sebmarkbage
reviewed
Aug 19, 2024
| chunks.set(id, createBlockedChunk(response)); | ||
| } else { | ||
| const blockedChunk: BlockedChunk<mixed> = (chunk: any); | ||
| blockedChunk.status = BLOCKED; |
There was a problem hiding this comment.
I think technically maybe you need to check if it is pending first. Because it could've maybe been rejected already if we aborted between the reference and this row.
using infinitely suspending promises isn't right because this will parse as a promise which is only appropriate if the value we're halting at is a promise. Instead we need to have a special marker type that says this reference will never resolve. Additionally flight client needs to not error any halted references when the stream closes because they will otherwise appear as an error
gnoff
force-pushed
the
refactor-flight-prerender
branch
from
35b6d73
to
6a12b85
Compare
sebmarkbage
added a commit
that referenced
this pull request
Aug 19, 2024
Stacked on #30731. When logging a Promise we emit it as an infinite promise instead of blocking the replay on it. This models that as a halted row instead. No need for this special case. I unflag the receiving side since now it's used to replace a feature that's already unflagged so it's used.
gnoff
added a commit
that referenced
this pull request
Aug 20, 2024
stacked on: #30731 We've refined the model of halting a prerender. Now when you abort during a prerender we simply omit the rows that would complete the flight render. This is analagous to prerendering in Fizz where you must resume the prerender to actually result in errors propagating in the postponed holes. We don't have a resume yet for flight and it's not entirely clear how that will work however the key insight here is that deciding whether the never resolving rows are an error or not should really be done on the consuming side rather than in the producer. This PR also reintroduces the logs for the abort error/postpone when prerendering which will give you some indication that something wasn't finished when the prerender was aborted.
eps1lon
added a commit
to vercel/next.js
that referenced
this pull request
Aug 23, 2024
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-1eaccd82-20240816` to `19.0.0-rc-eb3ad065-20240822`** No changes required in Next.js it seems. [diff facebook/react@1eaccd82...eb3ad065](facebook/react@1eaccd8...eb3ad06) <details> <summary>React upstream changes</summary> - facebook/react#30761 - facebook/react#30779 - facebook/react#30775 - facebook/react#30770 - facebook/react#30756 - facebook/react#30755 - facebook/react#30768 - facebook/react#30760 - facebook/react#30732 - facebook/react#30757 - facebook/react#30750 - facebook/react#30751 - facebook/react#30753 - facebook/react#30740 - facebook/react#30748 - facebook/react#30746 - facebook/react#30747 - facebook/react#30731 - facebook/react#30725 - facebook/react#30741 - facebook/react#30730 - facebook/react#30726 - facebook/react#30717 - facebook/react#30729 - facebook/react#30721 - facebook/react#30720 - facebook/react#30705 </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
using infinitely suspending promises isn't right because this will parse as a promise which is only appropriate if the value we're halting at is a promise. Instead we need to have a special marker type that says this reference will never resolve. Additionally flight client needs to not error any halted references when the stream closes because they will otherwise appear as an error
addresses: #30705 (comment)